Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report cause for Akka/IO TCP CommandFailed events #6221

Conversation

ismaelhamed
Copy link
Member

Port #22954

Changes

I think we should finally fix this once and forever. We have so many failed test jobs where we cannot be sure why "Connect failed" and we have no way of finding out without enabling DEBUG log level everywhere. That same pain is felt by every user.

Checklist

For significant changes, please ensure that the following have been completed (delete if not relevant):

public CommandFailed WithCause(Exception cause)
{
// Needs to be added with a mutable property for compatibility reasons
return new CommandFailed(Cmd) { Cause = cause };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this holds true in C# or we could just add a second constructor instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just add a constructor overload and that would work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although I think this is probably a fine way of doing it

@ismaelhamed ismaelhamed force-pushed the include-cause-in-command_failed-22954 branch from 8aa5df2 to efc6069 Compare November 1, 2022 09:54
Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb Aaronontheweb merged commit c9c0b44 into akkadotnet:dev Nov 1, 2022
@Aaronontheweb Aaronontheweb added this to the 1.5.0 milestone Nov 1, 2022
@Aaronontheweb
Copy link
Member

@Arkatufus can we backport to v1.4?

Aaronontheweb pushed a commit that referenced this pull request Nov 3, 2022
…6224)

* Port [#22954](akka/akka#22954)

## Changes

> I think we should finally fix this once and forever. We have so many failed test jobs where we cannot be sure why "Connect failed" and we have no way of finding out without enabling DEBUG log level everywhere. That same pain is felt by every user.

(cherry-picked from c9c0b44)

* Update API Verify list

Co-authored-by: Ismael Hamed <1279846+ismaelhamed@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants